Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Adding transitions to Suspense #154

Closed
wants to merge 12 commits into from
Closed

Conversation

jimmy-e
Copy link

@jimmy-e jimmy-e commented Apr 13, 2020

This RFC originated from this issue and proposes enhancing the suspense api such that it can enable transition effects for the fallback and children components by communicating to these components when suspense is "waiting". This will allow these components to render transitions when they mount and unmount by using libraries such as Framer Motion, React Spring, React Transition Group, and more.

View formatted RFC

@jamesplease
Copy link

Cross-linking #128 , which seems similar in intent

@gorkamolero
Copy link

How do we follow progress on this issue?

@escaton
Copy link

escaton commented Jun 1, 2020

Hi!
First of all, thank you for pushing this problem. This feature would be a gamechanger!

But there is one thing that bothers me: you actively use React.cloneElement to pass down the isWaiting prop. Although this is absolutely ok in pure javascript, it adds need for unnecessary dancing with typescript (and most probably flow too).
The children component (AnimationWrapper in your examples) needs to be declared with isWaiting as optional prop in order to be called in this manner <AnimationWrapper/>.
The solution is pretty straight forward: use render prop, like in Context.Consumer

There is another thing i'd like to discuss: whether unmount transition should be interruptible?
Imagine a nested hoverable dropdown: we could unmount its elements once mouse is out, but this would be a terrible UX. Instead we could add a delay before unmounting and allow user to quickly getting back to dropdown nested element.
That is something which requires additional logic in parent of such element now.
But this could be as simple as immediate deleting from parent render and adding back on mouse enter event. What we need is React to updating suspended component instead of creating new one. And I believe key can help us here :)

@bricejar
Copy link

This would be amazing.

@gaearon
Copy link
Member

gaearon commented Aug 23, 2021

In the spirit of #182, I'll provide some cursory comments about this proposal.

It's a bit challenging to read because the proposal mixes the proposed API with the "demo implementation", and it's a bit hard to tell where the parts you're proposing end, and the parts that were necessary to get the sandboxes running begin. I'm going to take a guess that the core of the proposal is the <Suspense> API itself, and everything around it is more like scaffolding.

With that assumption, "Proposal 1" looks unclear and incomplete. It doesn't seem to solve on the API level what you set out to solve, because, indeed, it's unclear how a parent component would receive a child Suspense status. (What is the API it would use? A proposal should specify a hypothetical one.) You call that out in the drawbacks. I think we can ignore this proposal because it doesn't propose a concrete API.

"Proposal 2" is clearer. Essentially, you're proposing this:

<Suspense
  fallback={<Fallback />}
  Wrapper={SomeWrapper} // <--- this is the proposal
>
  <Child />
</Suspense>

There are some nitpicks, like the fact that SomeWrapper is a component but fallback is an element. Or that SomeWrapper would have to use cloneElement which seems like a yellow flag for API design (e.g. typing difficulties, reliance on introspection). But we could simplify the proposal to mitigate this:

"Proposal 3"

<Suspense>
  {isWaiting => ( // <---- this is the proposal
    <Wrapper isWaiting={isWaiting}>
      {isWaiting ? <Fallback /> : <Child />}
    </Wrapper>
  )}
</Suspense>

This render prop API, in my opinion, is the substance of this RFC. (Btw, fun fact: if I recall correctly, this is also the API we started with. That was the initial Suspense API that we removed later.)

Currently, even if we switched the API to be like this, I don't think it would be sufficient for third-party animation solutions to work. This is because React handles switching between the fallback tree and the content tree differently from a regular component update. Specifically, switching between them (intentionally!) doesn't destroy the DOM. E.g. when you switch from content to fallback due to re-suspending, we hide (not remove!) the content, insert the fallback, and later show the content. This is different from unmounting in that we preserve the component state and the DOM state (e.g. a video player or a text input). Even if we exposed a render prop-like API, a Framer Motion or React Transition Group wrapper wouldn't be able to do the same thing. In addition, we wouldn't be able to distinguish between what to hide and what to remove because the information about them being separate trees is lost. This type of API (just as Proposal 2) is "too powerful" and loses the distinction between the content tree and the fallback tree, which are conceptually two separate trees with different behaviors.

In general, the idea that there needs to be an API controlling enter/exit for Suspense makes sense though. But it's also very limiting to implement this just for Suspense. Enter/exit animations are in general a very compelling feature, and third-party solutions are limited in what they can do. E.g. there is a common limitation in that the node being removed has to be a direct descendant of the wrapper component like <AnimatePresence>. This is not composable and doesn't match how the rest of React works, so fixing this is very desirable. After we finish our work on adding concurrency to React, animations are one of the next areas we will look closely at. So it probably doesn't make sense to add a very limited solution if we're going to revisit this whole space anyway with a much more generic solution.

As for how a generic solution would look like, we're not sure yet. But it probably follows a similar shape to what I outlined above, except it wouldn't be tied to Suspense. Something we've been discussing is a component like this:

<Presence>
  {status => // Enum: "visible" | "enter" | "exit"
    /* render anything */
  }
</Presence>

Crucially, this wrapper is NOT meant to wrap the deleted/inserted children, but the other way around. It's about specifying "what should be rendered as this subtree is being added/removed due to a change from a parent". E.g. in a list, an <Item> component would render a <Presence>, obtain status from it, and decide on its styles (e.g. for enter/exit). And the parent would just render an array of <Item>s, no special wrapper. React would diff the trees and orchestrate the necessary enter/exit animations. This makes animation compositional: if you want to add an enter/exit animation, you add it in the component that is being inserted/removed, without touching the parent components. It works across layers of abstraction so you can have as many components as you want in the middle. (However, note that removing parent <div> of an <Item> would not trigger the animation to prevent confusing cascades of animations. <Presence> is only taken into account upwards through userspace components directly to the point of deletion/insertion — but not further.)

This approach isn't specific to Suspense, but it would work for Suspense too, and would let us preserve the distinction between content/fallback trees, and the special behavior when switching them.

<Suspense fallback={<Fallback />}>
  <Child />
</Suspense>

function Fallback() {
  return (
    <Presence>
      {status => <Spinner opacity={status === 'exit' ? 0 : 1} />}
    </Presence>
  )
}

function Child() {
  // ... could also use Presence ...
}

This would let you do something like a crossfade between content and fallback. We'd also want to provide built-in support for layout animation which would let you automatically animate their parent box. But animations are a much broader topic in general, and it will be a big effort to design the API and implement it. I'm happy to keep this RFC open for discussion but I hope this context explains how we're thinking about these features and how they fit together in the long term.

@jimmy-e
Copy link
Author

jimmy-e commented Aug 23, 2021

Thank you Dan for all the feedback. I will close this PR as it seems apparent that this issue will be better addressed in future React api releases that are agnostic of Suspense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants